Skip to content

Conversation

@George-Guryev-flxcmp
Copy link
Contributor

@George-Guryev-flxcmp George-Guryev-flxcmp commented Oct 24, 2025

This PR introduces the first two automation utilities described in the corresponding Jira ticket aimed at simplifying model setup for lumped port simulations.

Added functions:

LumpedPort.from_structures() – Creates a lumped port by specifying a port plane (e.g. x = -L) together with the signal and ground structures. This helps automate port definition and reduce repetitive setup steps.

Simulation.padded_copy() – Adds padding layers automatically along simulation boundaries. Follows a similar specification style to BoundarySpec, supporting both uniform and side-specific padding.

Simulation.uniformly_padded_copy() - Applies padding uniformly along all simulation boundaries.

Motivation:
These utilities streamline the setup process for common notebook workflows involving lumped ports and improve consistency across examples.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/components/geometry/base.py (100%)
  • tidy3d/components/simulation.py (100%)
  • tidy3d/plugins/smatrix/ports/rectangular_lumped.py (88.9%): Missing lines 360,402,417,437,461,478,484,507-508

Summary

  • Total: 105 lines
  • Missing: 9 lines
  • Coverage: 91%

tidy3d/plugins/smatrix/ports/rectangular_lumped.py

Lines 356-364

  356         normal_axis, normal_coord = Geometry.parse_xyz_kwargs(x=x, y=y, z=z)
  357 
  358         # make sure voltage axis was specified
  359         if voltage_axis is None:
! 360             raise ValueError(
  361                 "No voltage axis was provided. Please make sure a valid voltage axis is specified."
  362             )
  363 
  364         # make sure normal is orthogonal to voltage axis

Lines 398-406

  398         voltage_axis_2d = 1 - lateral_axis_2d
  399 
  400         # Check: structures intersects plane
  401         if len(grounds_2d) == 0:
! 402             raise ValueError("Ground structure does not intersect Lumped Port plane.")
  403         if len(signals_2d) == 0:
  404             raise ValueError("Signal structure does not intersect Lumped Port plane.")
  405 
  406         # Get 2 element list with index 0 = ground shape and index 1 = signal shape

Lines 413-421

  413             # If lateral_coord is specified, pick ground/signal shape based on intersection
  414             if lateral_coord is not None:
  415                 # constract a line along voltage axis in the lumped port plane going throung the lateral coordinate
  416                 if voltage_axis_2d == 0:
! 417                     line = Geometry.make_shapely_box(-np.inf, lateral_coord, np.inf, lateral_coord)
  418                 else:
  419                     line = Geometry.make_shapely_box(lateral_coord, -np.inf, lateral_coord, np.inf)
  420 
  421                 # collect all geometries in shapes that intersect the line

Lines 433-441

  433                 sel_shape = union_all(shapes)
  434 
  435             # Check Lumped Port plane intersects a terminal
  436             if sel_shape.is_empty:
! 437                 raise ValidationError(
  438                     f"{name} terminal is not intersecting the lumped port plane or lateral coordinate."
  439                     "Please make sure the lumped port plane and/or terminals are specified correctly."
  440                 )
  441             if isinstance(sel_shape, BaseMultipartGeometry):

Lines 457-465

  457         ground_2d, signal_2d = shape_list
  458 
  459         # ensure that signal and ground terminals do not intersect
  460         if ground_2d.intersects(signal_2d):
! 461             raise ValidationError(
  462                 "Ground intersects signal in the specified plane."
  463                 "Please make sure that ground and signal terminals do not overlap."
  464             )

Lines 474-482

  474             return not (a_max < b_min or a_min > b_max)
  475 
  476         # ensure that terminal bounding boxes don't overlap along voltage axis
  477         if intervals_overlap(ground_bounds[voltage_axis_2d], signal_bounds[voltage_axis_2d]):
! 478             raise ValidationError(
  479                 "Auto-generation of lumped port failed because ground and signal terminals have overlapping bounds along voltage axis. "
  480                 "Please define lumped port manually."
  481             )
  482         # ensure that terminal bounding boxes overlap in lateral direction

Lines 480-488

  480                 "Please define lumped port manually."
  481             )
  482         # ensure that terminal bounding boxes overlap in lateral direction
  483         if not intervals_overlap(ground_bounds[lateral_axis_2d], signal_bounds[lateral_axis_2d]):
! 484             raise ValidationError(
  485                 "Auto-generation of lumped port failed because ground and signal terminals "
  486                 "don't have overlapping bounds along lateral axis. "
  487                 "Please define lumped port manually."
  488             )

Lines 503-512

  503                     lateral_coord + port_width / 2,
  504                 )
  505             else:
  506                 # If only port_width is specified, set based on bounds
! 507                 lcenter = (lmin + lmax) / 2
! 508                 lmin_new, lmax_new = (lcenter - port_width / 2, lcenter + port_width / 2)
  509                 # make sure that port_width does not exceed width of terminal overlap along lateral axis.
  510 
  511             if lmin_new < lmin or lmax_new > lmax:
  512                 raise ValueError(

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/lumped_port_setup branch 4 times, most recently from 7ebe90f to f3703b6 Compare October 25, 2025 00:06
Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for those nice convenient features! Two main comments:

  • When both lateral_coord and port_width are specified, it appears that ground and signal structures are not needed. Maybe we should move this to a separate method, e.g. from_lateral_dimension. As for from_structures, port_width option is good to have, but lateral_coord is not needed;
  • The number of shapes from intersections_plane can often be more than one, in particular if GeometryGroup or ClipOperation is used. Maybe we can merge shapes that intersect (e.g. shapely's union_all), and pick the nearest shapes between ground and signal to define the lumped port.

Copy link
Contributor

@dbochkov-flexcompute dbochkov-flexcompute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, added a few comments/suggestions.

Also, it might make sense to name padding functions as padded_copy/uniformly_padded_copy (or copy_with_padding/copy_with_uniform_padding, or even add_padding_copy/add_uniform_padding_copy) to emphasize we are not modifying simulation object in place and be consistent with other functions like updated_copy, perutbed_mediums_copy, symmetry_expanded_copy, etc

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/lumped_port_setup branch 4 times, most recently from 27b592c to b184600 Compare October 29, 2025 19:49
@George-Guryev-flxcmp
Copy link
Contributor Author

When both lateral_coord and port_width are specified, it appears that ground and signal structures are not needed. Maybe we should move this to a separate method, e.g. from_lateral_dimension. As for from_structures, port_width option is good to have, but lateral_coord is not needed.

I disagree: the specified lateral_coord and port_width do not guarantee that the signal and ground terminals exist at specified lateral_coord nor that their overlap along lateral axis contains the desired lumped port. One needs to have both structures to ensure validity of a lumped port constructed from lateral_coord and port_width.

The number of shapes from intersections_plane can often be more than one, in particular if GeometryGroup or ClipOperation is used. Maybe we can merge shapes that intersect (e.g. shapely's union_all), and pick the nearest shapes between ground and signal to define the lumped port.

Good point!

Copy link
Contributor

@dbochkov-flexcompute dbochkov-flexcompute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple more very minor comments from me

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/lumped_port_setup branch 2 times, most recently from b85170e to c3d824d Compare October 30, 2025 16:42
@weiliangjin2021
Copy link
Collaborator

When both lateral_coord and port_width are specified, it appears that ground and signal structures are not needed. Maybe we should move this to a separate method, e.g. from_lateral_dimension. As for from_structures, port_width option is good to have, but lateral_coord is not needed.

I disagree: the specified lateral_coord and port_width do not guarantee that the signal and ground terminals exist at specified lateral_coord nor that their overlap along lateral axis contains the desired lumped port. One needs to have both structures to ensure validity of a lumped port constructed from lateral_coord and port_width.

You are right. I missed this part!

@George-Guryev-flxcmp George-Guryev-flxcmp force-pushed the george/lumped_port_setup branch 2 times, most recently from 59f5786 to 4f75613 Compare October 31, 2025 02:04
Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to send the last comment

Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All look good to me!

@George-Guryev-flxcmp George-Guryev-flxcmp added this pull request to the merge queue Oct 31, 2025
@George-Guryev-flxcmp George-Guryev-flxcmp removed this pull request from the merge queue due to a manual request Nov 1, 2025
@George-Guryev-flxcmp George-Guryev-flxcmp added this pull request to the merge queue Nov 1, 2025
Merged via the queue into develop with commit 5ecb369 Nov 1, 2025
26 checks passed
@George-Guryev-flxcmp George-Guryev-flxcmp deleted the george/lumped_port_setup branch November 1, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants